Add go-expect based integration test for password resets#127
Add go-expect based integration test for password resets#127
Conversation
Askir
force-pushed
the
jascha/age-311-tiger-cli-investigate-interactive-testing-via-go-expect
branch
from
58b5d75 to
26cacde
Compare
Askir
changed the title
| // Set up command with PTY | ||
| cmd := exec.Command(binaryPath, "db", "connect", serviceID) | ||
| cmd.Stdin = c.Tty() | ||
| cmd.Stdout = c.Tty() | ||
| cmd.Stderr = c.Tty() | ||
| cmd.Env = buildTestEnv(tmpDir, publicKey, secretKey) | ||
|
|
||
| if err := cmd.Start(); err != nil { | ||
| t.Fatalf("Failed to start command: %v", err) | ||
| } |
There was a problem hiding this comment.
Do we really need to build the binary and execute it "from outside" to do this kind of testing with go-expect? Like, it works, but I worry that it's a little roundabout and makes some things more difficult than they need to be.
Would it work to just call buildRootCmd (like we do here, for instance), and then set Stdin/Stdout/Stderr via Command.SetIn, Command.SetOut, and Command.SetErr? Then we could just execute commands like we currently do, without having to build the binary, etc. Did you try that approach already? Seems like it would be more in-line with our current patterns (and a little more idiomatic for testing Go code), but I'm not sure if there's some reason that it wouldn't work with go-expect? 🤔
There was a problem hiding this comment.
The main reason I did this is because I wanted to run it all in a subprocess to be able to wait for the output, send input to the programm etc.
However Claude recommended me to try a go routine instead for parallelization and that seems to work however now the TTY detection failed because we didn't consistently use the cobra commands passed in streams. I refactored that part, now you have more to review 😅
Askir
force-pushed
the
jascha/age-311-tiger-cli-investigate-interactive-testing-via-go-expect
branch
from
26cacde to
d0892b6
Compare
Askir
force-pushed
the
jascha/age-311-tiger-cli-investigate-interactive-testing-via-go-expect
branch
from
d0892b6 to
7cf6d38
Compare
- Replace generic `readString` with specific `readLine` and `readPassword` functions for clearer intent and simpler call sites - Rename `checkStdinIsTTY` to `checkOutputIsTTY` to accurately reflect that it checks the output stream for TTY detection - Fix parameter ordering in `selectPasswordRecoveryOption` to be consistent (in, out) with other functions - Integration tests now use `buildRootCmd()` with cobra's SetIn/SetOut instead of spawning a subprocess, enabling direct PTY interaction - Remove `time.Sleep` calls from integration tests, use proper `ExpectString` waits instead - Remove unused helper functions: `buildTigerBinary`, `findProjectRoot`, `buildTestEnv` Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2 participants
This also changes to prefer the TIGER_CONFIG_DIR env var over the keyring if it is set.
This was necessary to give an option for the integration test not use the system keyring, but I think it generally makes sense to do this?
Output looks like this: